Skip to content

DNM: test aws ccm byo sg nlb with CI against go 1.26 version#461

Draft
mfbonfigli wants to merge 8 commits intoopenshift:mainfrom
mfbonfigli:dnm/test-aws-ccm-byo-sg-nlb-pr153
Draft

DNM: test aws ccm byo sg nlb with CI against go 1.26 version#461
mfbonfigli wants to merge 8 commits intoopenshift:mainfrom
mfbonfigli:dnm/test-aws-ccm-byo-sg-nlb-pr153

Conversation

@mfbonfigli
Copy link
Copy Markdown

@mfbonfigli mfbonfigli commented May 7, 2026 •
edited by coderabbitai Bot
Loading

Do not merge

Test CI run of AWS CCM e2e tests against go 1.26

To make the tests work on go 1.26, required by the upstream AWS CCM, the following was done:

  • removed openshift-testsccm-aws-tests from go.work
  • updated vendor
  • bumped Dockerfile base image to one supporting go 1.26
  • changed Makefile cloud-controller-manager-aws-tests-ext to build without the go workspace
  • force injected with a replace directive a version of onsi-ginkgo inside test extension that is compatible with both k8s.io/kubernetes 1.36.0 and the openshift-eng/openshift-tests-extension dependency

Summary by CodeRabbit

  • Chores
    • Updated Go runtime to 1.26 and switched to the newer OpenShift 5.0 builder image for builds.
    • Refreshed AWS SDK and Kubernetes-related dependencies to align with current platform versions.
    • Updated CI build image tag to match the new builder.
    • Improved local build steps to ensure a clean module cache, workspace-disabled build, and output placement.

@openshift-ci openshift-ci Bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label May 7, 2026
@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci Bot commented May 7, 2026

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 7, 2026 •
edited
Loading

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: ec833c96-e2bb-4f90-9724-2f23b3648a24

📥 Commits

Reviewing files that changed from the base of the PR and between 1925d26 and 988d1bd.

📒 Files selected for processing (1)
  • Makefile
🚧 Files skipped from review as they are similar to previous changes (1)
  • Makefile

Walkthrough

Updated build environment and Go toolchain: bumped module to Go 1.26, upgraded many k8s and transitive dependencies to v0.36.0/v1.36.0, switched CI/Docker builder image to rhel-9-release-golang-1.26-openshift-5.0, and adjusted a Makefile test build to run with GOWORK=off and clean module cache.

Changes

Go Toolchain / Dependency and CI Image Upgrade

Layer / File(s) Summary
Module toolchain
openshift-tests/ccm-aws-tests/go.mod		 go directive updated to 1.26.0.
Direct dependency bumps
openshift-tests/ccm-aws-tests/go.mod		 Updated k8s.io/* modules to v0.36.0/k8s.io/kubernetes v1.36.0; bumped AWS SDK and other direct deps.
Indirect / replace alignment
openshift-tests/ccm-aws-tests/go.mod		 Refreshed many indirect dependencies (gRPC/protobuf, OpenTelemetry, etc.) and updated replace entries to match the new Kubernetes baseline; added github.com/fsnotify/fsnotify v1.9.0 // indirect.
CI / Docker builder image
.ci-operator.yaml, Dockerfile		 Builder image/tag changed from rhel-9-release-golang-1.25-openshift-4.22 to rhel-9-release-golang-1.26-openshift-5.0.
Build invocation / Makefile
Makefile		 cloud-controller-manager-aws-tests-ext target: added go clean -modcache, ensures mkdir -p ../bin, and runs GOWORK=off go build -mod=mod ... producing ../bin/cloud-controller-manager-aws-tests-ext.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Important

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

❌ Failed checks (1 error, 2 warnings)

Check name Status Explanation Resolution
Stable And Deterministic Test Names ❌ Error PR adds openshift-tests/ccm-aws-tests/e2e/aws/loadbalancer.go with dynamic test name. Line 47 uses fmt.Sprintf() for Describe statement, violating stable naming requirement. Replace fmt.Sprintf with static string in test title. Change Describe(fmt.Sprintf("%s NLB [OCPFeatureGate:%s]", ...) to Describe("[cloud-provider-aws-e2e-openshift] loadbalancer NLB [OCPFeatureGate:AWSServiceLBNetworkSecurityGroup]")
Microshift Test Compatibility ⚠️ Warning Tests in loadbalancer.go reference FeatureGate API (config.openshift.io/v1) unavailable on MicroShift. No MicroShift protection mechanisms are present. Add [apigroup:config.openshift.io] to test name or wrap IsFeatureEnabled() with MicroShiftCluster() check. Alternatively add [Skipped:MicroShift] label.
Topology-Aware Scheduling Compatibility ⚠️ Warning New deployments have required pod anti-affinity and nodeSelector targeting master nodes. These constraints break on SNO, Two-Node, and HyperShift topologies. Use preferred anti-affinity instead of required. Avoid master-only nodeSelectors on HyperShift. Test with SNO, Two-Node, and HyperShift CI jobs.
✅ Passed checks (9 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title clearly identifies the main change: testing AWS CCM with Go 1.26, which aligns with the key modifications across Dockerfile, go.mod, and CI configuration.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Test Structure And Quality ✅ Passed Ginkgo tests follow single responsibility, use BeforeEach/DeferCleanup, include timeouts (wait.PollUntilContextTimeout, Eventually), and all assertions have meaningful messages.
Single Node Openshift (Sno) Test Compatibility ✅ Passed New Ginkgo e2e tests in loadbalancer.go do not assume multi-node clusters. All 6 tests (NLB config, service creation, ingress, update, cleanup, security rules) work on SNO deployments.
Ote Binary Stdout Contract ✅ Passed No stdout writes at OTE binary process level. All logging uses logrus (stderr) or framework.Logf (safe). No init(), main(), or BeforeSuite issues detected.
Ipv6 And Disconnected Network Test Compatibility ✅ Passed PR adds new Ginkgo e2e tests. No IPv4 hardcoded addresses, IPv4-only parsing, CIDR blocks, or external connectivity. Tests use only cluster-internal APIs and resources.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci Bot commented May 7, 2026

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign nrb for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@mfbonfigli
Copy link
Copy Markdown
Author

mfbonfigli commented May 7, 2026

/testwith openshift/cloud-provider-aws/main/e2e-aws-ovn-techpreview openshift/cloud-provider-aws#153 openshift/installer#10512

@mfbonfigli
Copy link
Copy Markdown
Author

mfbonfigli commented May 7, 2026

/testwith openshift/cloud-provider-aws/main/e2e-aws-ovn-techpreview openshift/cloud-provider-aws#153 openshift/installer#10512

@mfbonfigli
Copy link
Copy Markdown
Author

mfbonfigli commented May 7, 2026

/testwith openshift/cloud-provider-aws/main/e2e-aws-ovn-techpreview openshift/cloud-provider-aws#153 openshift/installer#10512

1 similar comment
@mfbonfigli
Copy link
Copy Markdown
Author

mfbonfigli commented May 8, 2026

/testwith openshift/cloud-provider-aws/main/e2e-aws-ovn-techpreview openshift/cloud-provider-aws#153 openshift/installer#10512

@mfbonfigli
Copy link
Copy Markdown
Author

mfbonfigli commented May 8, 2026

/testwith openshift/cloud-provider-aws/main/e2e-aws-ovn-techpreview openshift/cloud-provider-aws#153 openshift/installer#10512

@mfbonfigli
Copy link
Copy Markdown
Author

mfbonfigli commented May 8, 2026

/testwith openshift/cloud-provider-aws/main/e2e-aws-ovn-techpreview openshift/cloud-provider-aws#153 openshift/installer#10512

@mfbonfigli
Copy link
Copy Markdown
Author

mfbonfigli commented May 8, 2026

/testwith openshift/cloud-provider-aws/main/e2e-aws-ovn openshift/cloud-provider-aws#153 openshift/installer#10512

@mfbonfigli
Copy link
Copy Markdown
Author

mfbonfigli commented May 8, 2026

/testwith abort

@mfbonfigli
Copy link
Copy Markdown
Author

mfbonfigli commented May 8, 2026

/testwith openshift/cloud-provider-aws/main/e2e-aws-ovn openshift/cloud-provider-aws#153 openshift/installer#10512

@mfbonfigli
Copy link
Copy Markdown
Author

mfbonfigli commented May 8, 2026

/testwith openshift/cluster-cloud-controller-manager-operator/main/e2e-aws-ovn openshift/cloud-provider-aws#153 openshift/installer#10512

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@mfbonfigli